Create GH Actions workflow for build&test of Go client #2318

michalinacienciala · 2021-02-08T09:34:21Z

As described in RFC-18, there is a need for a refactorization of Keep
and tBTC release processes in order to reduce human involvment and make
the processes faster and less error prone. A part of this task is
migration from CircleCI jobs to GitHub Actions. This commit creates a
GitHub Action workfow for building and testing Go client.

As described in RFC-18, there is a need for a refactorization of Keep and tBTC release processes in order to reduce human involvment and make the processes faster and less error prone. A part of this task is migration from CircleCI jobs to GitHub Actions. This commit creates a GitHub Action workfow for building and testing Go client.

.github/workflows/client.yml

In order to improve workflow readability, notation of docker commands has been modified (long commands have been split to multiple lines). Also a way to access file containing unit tests results has been simplified (EnricoMi/[email protected] action requires access to file via a relative path).

Previous conditions for workflow kick-off [push, pull-request] were causing start of two simultaneous workflows when new commit was pushed to an already open pull request. After the change such situation should start only one workflow.

Each workflow should be malually triggerable and should trigger its downstream dependencies. This commit adresses the first part of that statement. Triggering of downstream dependencies is not yet developed, as downstream workflows aren't yet created.

pdyraga · 2021-02-09T09:34:49Z

What do you think - and is it possible - to disable the bot to post a new comment with test results after each push? IMO it is cluttering communication and all the most recent test results are available from the checks list.

github-actions · 2021-02-09T09:39:04Z

Go Test Results

0 files 0 suites 0s ⏱️
0 tests 0 ✔️ 0 💤 0 ❌

Results for commit 5affb78.

In order not to flood the PRs with lot of comments, publishing of test results to PR comments has been turned off. Results can be sill viewed by going to Checks tab. Commit also includes changes in the way test results are accessed (now GITHUB_WORKSPACE env variable is utilized).

.github/workflows/client.yml

Name of the step was incorrect - we do not enter the directory as part of the step. Co-authored-by: Jakub Nowakowski <[email protected]>

Change introduced to deliberatly make the tests fail. Will be reverted soon.

This reverts commit c41fb94.

nkuba

Just one tiny suggestion. Overall it looks very good to me.

nkuba · 2021-02-09T17:50:06Z

.github/workflows/client.yml

+            --tag keep-client . 
+      - name: Create test results directory
+        run: |
+          mkdir -p $GITHUB_WORKSPACE/test-results/


Maybe we could simplify it and do just mkdir test-results? In the next step, we've got to use $GITHUB_WORKSPACE to define the volume as a path, but I believe we don't need it here.

Command run in the step creating test results directory has been simplified. There is no need to use GITHUB_WORKSPACE env variable here as commands executed via 'run' keyword are executed in that location by default.

michalinacienciala · 2021-02-10T08:14:59Z

I've noticed one problem with EnricoMi/publish-unit-test-result-action@v1 action used for publishing of tests result - when a commit causes two separate workflows to trigger in parallel, the test results in the form of a 'check' are sometimes getting attached to incorrect workflow (i.e. to the one that did not use the EnricoMi/publish-unit-test-result-action@v1 action).
This is a bug that creators of the action are aware of (EnricoMi/publish-unit-test-result-action#12). I subscribed to that ticket, so I should get notified once this is fixed.

Added comment highlighting piece of CircleCI config code for which a GitHub Actions workflow has been already created (as part of work on RFC-18 that requires migration of build processes from CircleCI to GitHub Actions).

Shadowfiend · 2021-02-10T15:25:17Z

.github/workflows/client.yml

+on:  
+  push:
+    branches:
+      - master


One thing that may be worth considering here, if we're continuing to run the Circle build, is to change this filter to run on certain branches (e.g. rfc-18/*) and change Circle to not run on that filter. This effectively gives you a branch namespace to use for the work.

Implemented in #2329

Shadowfiend · 2021-02-10T15:26:36Z

.github/workflows/client.yml

+    steps:
+      - uses: actions/checkout@v1
+      - uses: satackey/[email protected]
+        continue-on-error: true # ignore the failure of a step and avoid terminating the job


I'll flag that the comment here mostly seems to be repeating what the flag already communicates (unlike if: always() which isn't as obvious).

You're right. I'm not changing it though, as this fragment of code will be removed once #2331 gets merged to master.

Shadowfiend · 2021-02-10T18:53:16Z

.github/workflows/client.yml

+          github_token: ${{ secrets.GITHUB_TOKEN }}
+          files:  ./test-results/unit-tests.xml
+          check_name: Go Test Results # name under which test results will be presented in GitHub (optional)
+          comment_on_pr: false # turns off commenting on Pull Requests


If things are working correctly, I believe the check should already be bubbling the important parts of this information up, including line annotations, e.g. like https://github.com/keep-network/keep-ecdsa/actions/runs/555260607 . I don't think we need the junit intermediary here, see https://github.com/actions/toolkit/blob/master/docs/commands.md#problem-matchers and the setup-go action's problem matcher at https://github.com/actions/setup-go/blob/main/matchers.json .

(Of note: we may want to add our own matcher that works with our go-to test output.)

Shadowfiend · 2021-02-10T18:53:57Z

Re: temporary commits to test failures: when reasonable, this is a great place to use reset + force-push to clear the commit away completely while the PR is still in draft, rather than clutter history with a test + revert. Another option is to have a separate branch for testing that runs parallel to your primary.

It's also imperfect, but worth looking at https://github.com/nektos/act, which provides limited support for running GitHub Actions locally.

michalinacienciala requested a review from nkuba February 8, 2021 09:34